perf: keep verified DKG contributions cached through the commit phase by knst · Pull Request #8135 · dashpay/dash

knst · 2026-11-12T08:12:59Z

Issue being fixed or feature implemented

Building a premature commitment for llmq_400_85 takes about 5 seconds of single-threaded work, almost all of it avoidable. SendCommitment and the verification of every received premature commitment call CDKGSessionManager::GetVerifiedContributions, which reads each member's verification vector back from LevelDB and decodes it: 400 members x 340 BLS public keys, about 34 µs per key (25 µs subgroup check, 8 µs point decompression). The contributions cache was filled only by reads and expired after 60 seconds, so it was always cold at commit time, although the DKG session still held the same vectors in memory.

#8130 reports that this delay made members miss short commit phases on mainnet, and fixes it. This PR is a smaller alternative to the cache part of #8130.

What was done?

WriteVerifiedVvecContribution now also puts the vector into contributionsCache, sharing the pointer the session already holds (no copy).
WriteVerifiedSkContribution updates the cached entry, so a share verified or justified later is visible to the next lookup.
CleanupCache drops an entry after 60 seconds only when nothing else references its vector, so entries live as long as their session and entries loaded from disk expire as before.
Benchmarks for the commit step at 400 members / threshold 340: BLSDKG_BuildCommitment_cold_400, BLSDKG_BuildCommitment_cached_400, BLSDKG_VerifyCommitment_400.

The database writes and the cold-read path are unchanged, so restarts, watchers and later quorum builds behave as before.

Compared to #8130

#8130 has the same idea but is +580 −9 across 7 files. This PR is +88 −6 across 3 files and reaches the same speed.

One behaviour difference from #8130: under the legacy BLS scheme (before v19) a received vector is cached as decoded, without the subgroup check that the database read applies. #8130 falls back to the database read in that case. There's no reason to keep different behaviour for 2 scenarios.

How Has This Been Tested?

Measurements

x86_64, blst, release build, 400 members / threshold 340:

Benchmark	Time
`BLSDKG_BuildCommitment_cold_400` (decode from database encoding first)	5.28 s
`BLSDKG_BuildCommitment_cached_400`	0.09 s
`BLSDKG_VerifyCommitment_400` (per received commitment)	0.018 s

Through the real CDKGSessionManager, using the end-to-end benchmark from #8130: 5.03 s on develop, 0.20 s with this PR, 0.22 s on #8130, same commitment hash in all three.

Breaking Changes

None. No protocol, message, consensus or on-disk change.

Checklist:

I have performed a self-review of my own code
I have made corresponding changes to the documentation
I have assigned this pull request to a milestone (if you have permission to set one)

Builds and verifies a premature commitment for 400 members and threshold 340. The cold variant first decodes every verification vector from its database encoding, as GetVerifiedContributions does on a cache miss.

At commit time GetVerifiedContributions re-read and decoded every member's verification vector from LevelDB, although the session still held them in memory. Fill the cache when a verified contribution is written and keep an entry while its vector is still referenced. The commit step for 400 members and threshold 340 goes from 5.0 s to 0.2 s.

thepastaclaw · 2026-11-12T08:17:05Z

⛔ Final review complete — 1 blocking finding(s) (commit 898351d)

coderabbitai · 2026-11-12T08:19:33Z

No actionable comments were generated in the recent review.

Walkthrough

The DKG benchmark code now supports an explicit threshold and includes benchmarks for commitment building and verification. The session manager now caches verified vectors, updates cached secret-key contributions, and retains expired entries while they have references outside the cache.

Estimated code review effort: 3 (Moderate) | ~20 minutes

Possibly related PRs

dashpay/dash#8130: It changes caching and expiry behavior for verified DKG contributions in the same session-manager code path.

Merge Risk: Minimal · up to 89835

The change keeps verified DKG contribution vectors cached through the commit phase to avoid repeated database decoding, and no concrete behavioral defect was established. The remaining items are the PR title scope and milestone checks, which should be fixed before merge. A legacy-scheme (pre-V19) edge case was not fully traced, but the evidence gathered does not indicate a failure.

Pre-merge checks: 4

Check name	Status	Explanation
Linked Issues check		Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check		Check skipped because no linked issues were found for this pull request.
Title check		The title "perf: keep verified DKG contributions cached through the commit phase" matches the main change. The PR caches verified DKG contribution vectors in contributionsCache so that commitment buil…
Description check		The description is specific to the changeset. It explains that the DKG contributions cache is filled only by reads and expires after 60 seconds, so commitment building reads and decodes vectors from L…

Full details: Title check

Explanation

The title "perf: keep verified DKG contributions cached through the commit phase" matches the main change. The PR caches verified DKG contribution vectors in contributionsCache so that commitment building and verification do not reload and decode them from LevelDB. The title is short, specific, and states the primary performance goal. It does not cover the benchmarks or the cleanup rule, which is acceptable.

Full details: Description check

Explanation

The description is specific to the changeset. It explains that the DKG contributions cache is filled only by reads and expires after 60 seconds, so commitment building reads and decodes vectors from LevelDB. It lists the changes to WriteVerifiedVvecContribution, WriteVerifiedSkContribution and CleanupCache, the three new benchmarks at 400 members and threshold 340, and the measured timings. It also describes the legacy BLS scheme difference from #8130 and states that no protocol, consensus or on-disk changes are made. This matches the summary of changes in dkgsessionmgr.cpp, dkgsessionmgr.h and bls_dkg.cpp.

Finishing Touches

Generate unit tests (beta)

Create a new PR

Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

Share

Recent review info

Run configuration

Source mode: Auto

Repository: Unavailable

Branch: Unavailable

File: No configuration file from this repository applied

Commit: Unavailable

Layers: Repository UI

Review profile: CHILL

Plan: Advanced

Run ID: `7b2ce74e-1c06-4ccf-849b-43cc17091671`

Commits

Reviewing files that changed from the base of the PR and between `6f5e9cf` and `898351d`.

Files selected for processing: 3

`src/bench/bls_dkg.cpp`

`src/llmq/dkgsessionmgr.cpp`

`src/llmq/dkgsessionmgr.h`

Included review availability: This review used your included allowance. Your plan provides up to 4 included reviews per hour; 3 remain after this review.

PastaPastaPasta · 2026-11-12T08:39:38Z

Tested this for v24.0.1, where it replaces the cache part of #8130.

Read against the session code: the vector is always written on receipt and the secret-key share only afterwards (VerifyPendingContributions, justifications), so the share update always finds the entry; and an entry can only be evicted once the session no longer holds the vector. On a synced mainnet node the result is the same as a cold read.
The source commit (898351d) applies to v24.0.x with one conflict, in CleanupCache, where v24.0.x still has the hand-written erase loop; resolved by taking the new std::erase_if.
On that v24.0.x build (relic, --enable-debug --enable-werror): unit suites llmq*, dkg*, bls*, evo* pass; feature_llmq_dkgerrors.py, feature_llmq_simplepose.py (both variants), feature_llmq_rotation.py, feature_llmq_signing.py (both variants), feature_llmq_data_recovery.py, feature_llmq_connections.py, feature_llmq_chainlocks.py, feature_llmq_evo.py, feature_llmq_singlenode.py and feature_mnehf.py (both variants) pass.

#8128 now carries this commit. The "Require a milestone" check is failing here only because no milestone is set yet.

🤖 Posted autonomously by Claude on behalf of pasta.

thepastaclaw

Final validation — Phase 1 + Phase 2

Verified the changes at 898351d. The cache optimization is narrowly scoped, but it bypasses subgroup validation for legacy-decoded verification vectors, and the new cache lifecycle lacks targeted regression coverage. This was a static review; current CI reports passing build, test, lint, formatting, and policy checks, with only the automated review pending.

🔴 1 blocking | 🟡 1 suggestion(s)

Review provenance

Source: reviewer 1: glm-5.3-flash (agent: phase1-reviewer, role: general); reviewer 2: gpt-6.1-sol (agent: phase2-reviewer, role: general); reviewer 3: gpt-6.1-sol (agent: phase2-reviewer, role: dash-core-commit-history); final verifier: gpt-6.1-sol (agent: sol-verifier, role: final-verifier)

Triage: low by gpt-6.1-sol (effort low) — The diff is a small, contained cache-lifetime and cache-update change in CDKGSessionManager plus benchmarks, not a large or intricate modification to cryptographic or consensus rules.
Phase 1 reviewers: glm-5.3-flash — general (completed, effort high); agent phase1-reviewer
Phase 1 model: glm-5.3-flash — zai quota: 5h 94% left, weekly 68% left; passed over gemini-3.8-flash-high (antigravity quota lookup failed)
Fresh verifier: gpt-6.1-sol — final-verifier; agent sol-verifier
Phase 2 reviewers: gpt-6.1-sol — general (completed, effort medium); agent phase2-reviewer, gpt-6.1-sol — dash-core-commit-history (completed, effort medium); agent phase2-reviewer

🤖 Prompt for all review comments with AI agents

These findings are from an automated code review (PastaClaw). How to work through them:
- Verify each finding against the code; fix only what is real, as narrowly as the finding.
- If a finding is wrong, decline it on its thread in one or two sentences; the bot answers once and stops.
- To defer one, link a tracking issue on its thread (or reply asking the bot to file one).
- Suggestions don't block the merge, but most are worth fixing in this PR; judge each on its merits.
- Keep the PR's scope; don't grow it to satisfy hardening suggestions.
- Carried items were raised in an earlier round: read the thread first, and don't re-fix what you already fixed, declined or deferred there. Disputed and deferred items are already handled and are not listed.

In `src/llmq/dkgsessionmgr.cpp`:
- [BLOCKING] src/llmq/dkgsessionmgr.cpp:132-136: Preserve subgroup validation before caching legacy-decoded vectors
  This insertion bypasses a validation boundary when a contribution was decoded under the legacy BLS scheme. CDKGContribution::Unserialize permits that scheme, and G1Element::FromBytes explicitly skips CheckValid() for legacy decoding. Although the underlying G1Element::IsValid() checks curve and subgroup membership, CBLSPublicKey::IsValid() is a different method: it returns only the wrapper's fValid flag, which SetBytes sets after legacy decoding without checking subgroup membership. Both PreVerifyMessage's VerifyVerificationVector and BuildQuorumVerificationVector inspect that flag and duplicates, so neither supplies the missing check. Consequently, the new cache can return coefficients that the existing basic-scheme database reader rejects, making commitment reconstruction depend on whether the cache is populated. Legacy operation remains supported, including through the regtest v19 activation-height override. Pass the recorded decoding scheme into the write path and retain the cold-read behavior for legacy-decoded vectors, or perform equivalent validation before caching them.
- [SUGGESTION] src/llmq/dkgsessionmgr.cpp:240-242: Add regression coverage through the actual contribution cache
  The new benchmarks operate on local vectors and CBLSWorker; they never instantiate CDKGSessionManager or exercise its modified write, read, or cleanup paths. Existing functional DKG tests exercise successful commitments and justification, but do not directly assert ownership-dependent expiry or compare warm-cache results with disk-loaded results. Extend the existing DKG unit suite with manager-level checks that retain an aged entry while an external vector owner exists, expire it after that owner releases it, expose a subsequently written secret share, and preserve cached-versus-cold behavior. Include the legacy validation boundary described above so future performance changes cannot silently remove it again.

thepastaclaw · 2026-11-12T09:35:52Z

+    // Reading the vvec back at commit time costs a point decompression and subgroup check per coefficient
+    LOCK(contributionsCacheCs);
+    auto& entry = contributionsCache[{llmqType, pQuorumBaseBlockIndex->GetBlockHash(), proTxHash}];
+    entry.entryTime = SteadyClock::now();
+    entry.vvec = vvec;


🔴 Blocking: Preserve subgroup validation before caching legacy-decoded vectors

This insertion bypasses a validation boundary when a contribution was decoded under the legacy BLS scheme. CDKGContribution::Unserialize permits that scheme, and G1Element::FromBytes explicitly skips CheckValid() for legacy decoding. Although the underlying G1Element::IsValid() checks curve and subgroup membership, CBLSPublicKey::IsValid() is a different method: it returns only the wrapper's fValid flag, which SetBytes sets after legacy decoding without checking subgroup membership. Both PreVerifyMessage's VerifyVerificationVector and BuildQuorumVerificationVector inspect that flag and duplicates, so neither supplies the missing check. Consequently, the new cache can return coefficients that the existing basic-scheme database reader rejects, making commitment reconstruction depend on whether the cache is populated. Legacy operation remains supported, including through the regtest v19 activation-height override. Pass the recorded decoding scheme into the write path and retain the cold-read behavior for legacy-decoded vectors, or perform equivalent validation before caching them.

_{source: gpt-6.1-sol (phase2-reviewer: general)}

The difference is real but narrow. vvecLegacyScheme is only set while the node's tip is below the v19 height (mainnet 1899072, testnet 850100, devnets 2), so on mainnet and testnet a node taking part in DKGs never decodes a vector that way. It is reachable on regtest with -testactivationheight=v19@N.

The smallest change that keeps the old behaviour there is to pass qc.vvecLegacyScheme into WriteVerifiedVvecContribution and skip the cache write when it is set, so those vectors keep going through the DB read. The later WriteVerifiedSkContribution then finds no entry and is a no-op for the cache, which is already handled.

If useful, both are available to take from PastaPastaPasta/dash, branch backup/dkg-commit-contrib-cache-68de86bf: 96b68d0 plumbs that parameter (it does more than needed; only the parameter and the skip matter here), and 04e93df has manager-level unit tests for the cache (covers the second finding, though some cases assume that commit's eviction rule and would need adjusting).

Whatever lands here, I'll redo the pick in #8128 to match.

🤖 Posted autonomously by Claude on behalf of pasta.

Agreed that the affected path is narrow, rather than a normal synced-mainnet DKG path. Passing qc.vvecLegacyScheme into WriteVerifiedVvecContribution and skipping cache insertion after the existing DB write when it is true resolves this concern: those vectors retain the basic-scheme cold-read validation, and WriteVerifiedSkContribution already handles a cache miss safely. I’m satisfied with that minimal fix; the additional hash and eviction changes in 96b68d07e88e aren’t needed for this finding. This remains pending until the fix lands on the PR.

The finding stands on this commit until that change is pushed; the push gets a full review.

Following up on my comment above after looking at when the flag can actually be legacy: I don't think this should block.

vvecLegacyScheme is only set if the process-wide flag is legacy when the contribution is decoded. That needs a tip below the v19 height, or a background chainstate connecting pre-v19 blocks under a snapshot, and mainnet and testnet have no snapshot parameters. A node in that state is not taking part in DKGs with the rest of the network either way.

The re-check this removes was a side effect of reading the vector back from the DB in basic format, not a designed validation step. Where the session needs to know about legacy-decoded vectors it already does so explicitly (anyLegacyVvec in VerifyPendingContributions).

Passing the flag into WriteVerifiedVvecContribution would thread more legacy-scheme state through the manager just before the planned removal of the BLS globals. The better place to close this is at decode time, as part of that work, so a DKG message is never decoded under the legacy scheme at all.

So: fine to merge as is from my side; the suggestion above can be ignored.

🤖 Posted autonomously by Claude on behalf of pasta.

I don't think the activation argument removes the issue: the DKG session code itself permits pre-v19 sessions (UpdatedBlockTip and the handler gate on DIP0003, while ActiveDKGSession sets m_use_legacy_bls before v19). In that path, PreVerifyMessage checks only CBLSPublicKey::IsValid(), which is the wrapper flag, and WriteVerifiedVvecContribution immediately places the legacy-decoded vector in the cache; the old commit-time DB read instead re-decodes the stored basic encoding and performs the subgroup check. Thus the cache changes behavior for a supported legacy/regtest path, regardless of whether a currently synced mainnet node reaches it, and the proposed future removal of the global scheme does not make this commit behavior-preserving.

thepastaclaw · 2026-11-12T09:35:52Z

+    // A vvec still referenced elsewhere (by its DKG session) costs nothing to keep and is expensive to reload
+    std::erase_if(contributionsCache, [&](const auto& entry) {
+        return entry.second.vvec.use_count() == 1 && curTime - entry.second.entryTime > MAX_CONTRIBUTION_CACHE_TIME;


🟡 Suggestion: Add regression coverage through the actual contribution cache

The new benchmarks operate on local vectors and CBLSWorker; they never instantiate CDKGSessionManager or exercise its modified write, read, or cleanup paths. Existing functional DKG tests exercise successful commitments and justification, but do not directly assert ownership-dependent expiry or compare warm-cache results with disk-loaded results. Extend the existing DKG unit suite with manager-level checks that retain an aged entry while an external vector owner exists, expire it after that owner releases it, expose a subsequently written secret share, and preserve cached-versus-cold behavior. Include the legacy validation boundary described above so future performance changes cannot silently remove it again.

_{source: gpt-6.1-sol (phase2-reviewer: general)}

PastaPastaPasta

Approved; thepastaclaw's concern is real; but non-blocking, will be resolved by future enhancements that remove legacy bls global

PastaPastaPasta

Approved; thepastaclaw's concern is real; but non-blocking, will be resolved by future enhancements that remove legacy bls global

…eer messages 0c3ba64 fix(llmq): process our own DKG messages ahead of queued peer messages (pasta) Pull request description: ## Issue being fixed or feature implemented A masternode's own DKG messages (contribution, complaint, justification, premature commitment) are pushed onto the same pending queue as the messages it receives from peers, at the back. They are only verified and relayed when the DKG thread gets to them, so a node's own message waits behind every peer message that was already queued. In the commit phase that wait can be long. Each received premature commitment is verified one at a time on the DKG thread (a public-key-share computation and a pairing: about 20 ms on blst, several times that on relic), and in a 400-member quorum up to 399 of them can be ahead. A slow member that falls behind on verifying its peers' commitments therefore also holds back its own, and the commit phase is 4 blocks, which fast blocks have cut to under two minutes on mainnet (see #8135 for the rounds that failed). The other members can only count a commitment they have received. This PR used to carry a contribution cache change as well. #8135 does that part in less code, so it has been dropped from here; this is the remaining, independent change. ## What was done? `CDKGPendingMessages::PushPendingMessage` puts a message the node produced itself (`from == -1`) at the front of the queue instead of the back. Peer messages keep their order. Nothing else changes: the message is still verified by the same code before it is relayed, and there is no protocol, message, consensus or on-disk change. ## How Has This Been Tested? - New unit test `pending_messages_own_message_is_processed_first` in `llmq_dkg_tests`: with peer messages queued, a node's own message is popped first and the peer messages follow in their original order. It fails with the change reverted. - Unit tests `llmq_dkg_tests` pass. - Functional tests on this branch: `feature_llmq_dkgerrors.py`, `feature_llmq_simplepose.py` (both variants), `feature_llmq_rotation.py`, `feature_llmq_signing.py` (both variants), `feature_llmq_data_recovery.py`, `feature_llmq_connections.py` and `feature_llmq_chainlocks.py` pass. - The same commit on a `v24.0.x` build together with #8135's cache commit: unit tests `llmq*`, `dkg*`, `bls*`, `evo*` pass, and the functional tests above plus `feature_llmq_evo.py`, `feature_llmq_singlenode.py` and `feature_mnehf.py` pass. The effect on a mainnet-sized commit phase was not measured; the change is justified by the queue order alone. ## Breaking Changes None. ## Checklist: - [x] I have performed a self-review of my own code - [ ] I have made corresponding changes to the documentation - [x] I have assigned this pull request to a milestone _(if you have permission to set one)_ 🤖 Generated with [Claude Code](https://claude.com/claude-code) ACKs for top commit: knst: utACK 0c3ba64 Tree-SHA512: c2078b24b75095470fc0549c4fa53396809a1cfb819c3fd25a38548ddded3cb03dcd609b380aa67ef11149422cf9a2d80ff1a6ea9e2d36061fa7e5a78a5455f8

…ugh the commit phase 898351d perf(llmq): keep verified DKG contributions cached for the session (Konstantin Akimov) 49d9ed5 bench: measure the DKG commit step at llmq_400_85 size (Konstantin Akimov) Pull request description: ## Issue being fixed or feature implemented Building a premature commitment for `llmq_400_85` takes about 5 seconds of single-threaded work, almost all of it avoidable. `SendCommitment` and the verification of every received premature commitment call `CDKGSessionManager::GetVerifiedContributions`, which reads each member's verification vector back from LevelDB and decodes it: 400 members x 340 BLS public keys, about 34 µs per key (25 µs subgroup check, 8 µs point decompression). The contributions cache was filled only by reads and expired after 60 seconds, so it was always cold at commit time, although the DKG session still held the same vectors in memory. dashpay#8130 reports that this delay made members miss short commit phases on mainnet, and fixes it. This PR is a smaller alternative to the cache part of dashpay#8130. ## What was done? - `WriteVerifiedVvecContribution` now also puts the vector into `contributionsCache`, sharing the pointer the session already holds (no copy). - `WriteVerifiedSkContribution` updates the cached entry, so a share verified or justified later is visible to the next lookup. - `CleanupCache` drops an entry after 60 seconds only when nothing else references its vector, so entries live as long as their session and entries loaded from disk expire as before. - Benchmarks for the commit step at 400 members / threshold 340: `BLSDKG_BuildCommitment_cold_400`, `BLSDKG_BuildCommitment_cached_400`, `BLSDKG_VerifyCommitment_400`. The database writes and the cold-read path are unchanged, so restarts, watchers and later quorum builds behave as before. ### Compared to dashpay#8130 dashpay#8130 has the same idea but is +580 −9 across 7 files. This PR is +88 −6 across 3 files and reaches the same speed. One behaviour difference from dashpay#8130: under the legacy BLS scheme (before v19) a received vector is cached as decoded, without the subgroup check that the database read applies. dashpay#8130 falls back to the database read in that case. There's no reason to keep different behaviour for 2 scenarios. ## How Has This Been Tested? ### Measurements x86_64, blst, release build, 400 members / threshold 340: | Benchmark | Time | |---|---:| | `BLSDKG_BuildCommitment_cold_400` (decode from database encoding first) | 5.28 s | | `BLSDKG_BuildCommitment_cached_400` | 0.09 s | | `BLSDKG_VerifyCommitment_400` (per received commitment) | 0.018 s | Through the real `CDKGSessionManager`, using the end-to-end benchmark from dashpay#8130: 5.03 s on develop, 0.20 s with this PR, 0.22 s on dashpay#8130, same commitment hash in all three. ## Breaking Changes None. No protocol, message, consensus or on-disk change. ## Checklist: - [x] I have performed a self-review of my own code - [ ] I have made corresponding changes to the documentation - [x] I have assigned this pull request to a milestone _(if you have permission to set one)_ Top commit has no ACKs. Tree-SHA512: fa65560612bbd1ac1d448a4e35cb1f3a4316257a5ff521f1cb503599c91a33ca221113d53f244b3db0d1ca1c4cf99de68f06118f6a6fcb25272f15b88b904136 (cherry picked from commit 1abaa0d)

…ueued peer messages 0c3ba64 fix(llmq): process our own DKG messages ahead of queued peer messages (pasta) Pull request description: ## Issue being fixed or feature implemented A masternode's own DKG messages (contribution, complaint, justification, premature commitment) are pushed onto the same pending queue as the messages it receives from peers, at the back. They are only verified and relayed when the DKG thread gets to them, so a node's own message waits behind every peer message that was already queued. In the commit phase that wait can be long. Each received premature commitment is verified one at a time on the DKG thread (a public-key-share computation and a pairing: about 20 ms on blst, several times that on relic), and in a 400-member quorum up to 399 of them can be ahead. A slow member that falls behind on verifying its peers' commitments therefore also holds back its own, and the commit phase is 4 blocks, which fast blocks have cut to under two minutes on mainnet (see dashpay#8135 for the rounds that failed). The other members can only count a commitment they have received. This PR used to carry a contribution cache change as well. dashpay#8135 does that part in less code, so it has been dropped from here; this is the remaining, independent change. ## What was done? `CDKGPendingMessages::PushPendingMessage` puts a message the node produced itself (`from == -1`) at the front of the queue instead of the back. Peer messages keep their order. Nothing else changes: the message is still verified by the same code before it is relayed, and there is no protocol, message, consensus or on-disk change. ## How Has This Been Tested? - New unit test `pending_messages_own_message_is_processed_first` in `llmq_dkg_tests`: with peer messages queued, a node's own message is popped first and the peer messages follow in their original order. It fails with the change reverted. - Unit tests `llmq_dkg_tests` pass. - Functional tests on this branch: `feature_llmq_dkgerrors.py`, `feature_llmq_simplepose.py` (both variants), `feature_llmq_rotation.py`, `feature_llmq_signing.py` (both variants), `feature_llmq_data_recovery.py`, `feature_llmq_connections.py` and `feature_llmq_chainlocks.py` pass. - The same commit on a `v24.0.x` build together with dashpay#8135's cache commit: unit tests `llmq*`, `dkg*`, `bls*`, `evo*` pass, and the functional tests above plus `feature_llmq_evo.py`, `feature_llmq_singlenode.py` and `feature_mnehf.py` pass. The effect on a mainnet-sized commit phase was not measured; the change is justified by the queue order alone. ## Breaking Changes None. ## Checklist: - [x] I have performed a self-review of my own code - [ ] I have made corresponding changes to the documentation - [x] I have assigned this pull request to a milestone _(if you have permission to set one)_ 🤖 Generated with [Claude Code](https://claude.com/claude-code) ACKs for top commit: knst: utACK 0c3ba64 Tree-SHA512: c2078b24b75095470fc0549c4fa53396809a1cfb819c3fd25a38548ddded3cb03dcd609b380aa67ef11149422cf9a2d80ff1a6ea9e2d36061fa7e5a78a5455f8 (cherry picked from commit 1e3aeb3)

78ad074 Merge #8131: fix: wait for a matching superblock trigger before voting NO on the others (merge-script) 6faa5ce Merge #8097: fix: keep governance triggers across a restart on test networks (pasta) 2757e83 Merge #8130: fix: process a node's own DKG messages ahead of queued peer messages (merge-script) cbbee0e Merge #8135: perf: keep verified DKG contributions cached through the commit phase (merge-script) 29a3cdd Merge #8099: fix(rest): don't terminate on blocks with undecodable ProTx payloads (merge-script) 275a142 Merge #8100: fix: don't abort in governance when a reorg changes masternode internal ids (merge-script) d00c4e6 Merge #8104: fix: keep final commitments mineable when their block is rejected (merge-script) b051d3a Merge #8080: test(qt): stop address book tests from answering the shared masternode wizard's prompt (merge-script) ec9e944 Merge #8094: fix(net): stop busy-looping while a peer's send buffer is full (merge-script) 033dae3 Merge #8052: fix(qt): reserve the same shared masternode contribution again after a release (merge-script) 65603d5 Merge #8088: fix: fetch miniupnpc sources from miniupnp.free.fr (merge-script) Pull request description: ## Issue being fixed or feature implemented Backports for v24.0.1, on top of `v24.0.x` at 938fba5 (v24.0.0). v24.0.0 is running on mainnet ahead of the V24 hard fork, and a handful of fixes that landed on develop since the release should not wait for 24.1: a `test_dash-qt` failure that blocks distribution packagers (#8075), a broken dependency download, a peer-triggerable busy loop, a way for a quorum commitment to be lost for a whole DKG cycle, and a node abort after a reorg. It also carries #8131, which changes when masternodes vote NO on superblock triggers. Inside a superblock's maturity window a masternode votes NO on every trigger that doesn't match its candidate and never revisits that vote. If the approved set changes while triggers are being created, masternodes can end up having voted NO on the trigger that matches their final candidate, and the superblock can go unpaid. With #8131 a masternode waits for a trigger matching its own candidate before it votes NO on the others (until 3 blocks before the superblock), and clears the per-cycle "voted YES" marker by height so an unfunded cycle doesn't carry over into the next one. This was a separate PR (#8132) until #8131 merged on develop; it is folded in here so the release branch gets one backport PR. It also carries the fixes for the `llmq_400_85` DKG rounds that have been failing on mainnet, which is what the stalled v24 activation is waiting on: #8135, which stops DKG members from reloading every verification vector at commit time, the reason slow members have been missing the commit phase of `llmq_400_85` rounds, and #8130, which makes a member relay its own DKG messages before it works through queued peer messages. ## What was done? ### Picked All eleven are picked from their develop merge commits with `git cherry-pick -m1 -x`. Each pick changes the same lines as its develop counterpart except where noted. - #8088 fix: fetch miniupnpc sources from miniupnp.free.fr - #8052 fix(qt): reserve the same shared masternode contribution again after a release - #8094 fix(net): stop busy-looping while a peer's send buffer is full - #8080 test(qt): stop address book tests from answering the shared masternode wizard's prompt (fixes #8075) - #8104 fix: keep final commitments mineable when their block is rejected, with the adaptation below - #8100 fix: don't abort in governance when a reorg changes masternode internal ids, with the adaptation below - #8099 fix(rest): don't terminate on blocks with undecodable ProTx payloads - #8135 perf: keep verified DKG contributions cached through the commit phase, with the adaptation below - #8130 fix: process a node's own DKG messages ahead of queued peer messages - #8097 fix: keep governance triggers across a restart on test networks, source change only (see below) - #8131 fix: wait for a matching superblock trigger before voting NO on the others ### Adaptations - **#8104:** `v24.0.x` doesn't have #7936 (parse each quorum commitment payload once per block). `src/llmq/blockprocessor.cpp` keeps the `v24.0.x` `GetCommitmentsFromBlock` and adds the new `RemoveMineableCommitmentsForBlock()` unchanged; in `blockprocessor.h` its lock annotation drops `!m_commitment_payload_mutex`, a mutex that only exists with #7936. `Chainstate` has no `m_special_tx` member on `v24.0.x`, so the two new calls in `validation.cpp` go through `m_chain_helper->special_tx`, like the neighbouring special-transaction calls. The `specialtxman.*` hunks are identical to the develop merge. - **#8135:** in `CDKGSessionManager::CleanupCache`, `v24.0.x` still has a hand-written erase loop that develop had already replaced with `std::erase_if`; the pick takes the new `std::erase_if` form. The benchmark added in `src/bench/bls_dkg.cpp` is left out: it conflicts with the `v24.0.x` version of that file and isn't needed on the release branch. The rest of the source change is identical. - **#8097:** only the source change is picked (two lines in `src/governance/governance.cpp`). It is needed for #8131's test: without it a masternode restarted inside the maturity window drops its stored triggers on regtest, testnet and devnets and then rejects the superblock, which is what the restart case of that test does. Mainnet is not affected by that bug, because the expiry check involved is skipped there. #8097's own `feature_governance.py` step is left out because it restarts the pruned node added with #7860, which `v24.0.x` doesn't have. - **#8131:** no conflicts; `src/governance/signing.cpp` on `v24.0.x` is identical to develop before the change. - **#8100:** the test fixture in `src/test/governance_vote_processing_tests.cpp` keeps `CBaseChainParams::REGTEST`, since `v24.0.x` predates `ChainType`. The source change is identical. ### Not picked - #8129 (re-sign a pending EHF signal on every block): an earlier revision of this PR carried it. It is still under discussion on develop, and v24.0.1 goes ahead without it: the EHF signal is attempted again when a newly formed quorum is selected for it, which the DKG fixes here are meant to make possible. - #8101 (reject InstantSend locks with a noncanonical version or cycle hash): an earlier revision of this PR carried it. It scores a peer 100 for relaying a lock with a non-current version, but nodes without the change accept and relay such a lock, so in a mixed network an upgraded node would discourage honest peers that are still on v24.0.0. It needs to stop scoring that case before it ships anywhere. - #8095 and its follow-up #8121 (block templates while the best ChainLock is above the tip): with both, a node refuses to build a template in that window, which is what v24.0.0 already does with a less helpful error. An earlier revision of this PR carried #8095 alone; it was dropped once #8121 merged. - #8102 (quorum scan cache): written on top of the #7956 refactor; it needs a hand-port to two functions on `v24.0.x`. - #8116 and #7815 (CoinJoin coordinator): long-standing, mixing only; #8116's test depends on #7815. - #8085, #8096, #8098, #8103, and the features, refactors, Bitcoin Core backports, CI and test-only changes merged since the branch point. ## How Has This Been Tested? macOS arm64, depends build with `--enable-debug --enable-werror`, no GUI. The current head is 78ad074. On the current head: - `dashd` and `test_dash` build cleanly. - Unit suites `governance*`, `llmq*`, `dkg*`, `evo*` and `versionbits*` pass, on the relic-based BLS library this branch uses. - `feature_governance_trigger_voting.py`, `feature_governance.py`, `feature_governance_cl.py`, `feature_governance_objects.py`, `feature_llmq_dkgerrors.py`, `feature_mnehf.py` and `interface_rest.py` pass. On earlier revisions of this branch. The current head has the same content as those revisions for the picks concerned; since then #8129 was dropped (one line in `src/llmq/ehf_signals.cpp` plus its test changes), the #8097 and #8131 picks were added, and the later picks were redone from their develop merge commits without changing the tree: - `feature_llmq_signing.py` (both variants), `feature_llmq_chainlocks.py` and `feature_llmq_rotation.py` pass (on the revision without #8097 and #8131). - Without the #8097 source change, `feature_governance_trigger_voting.py` fails in its restart case: the restarted masternode rejects the superblock with `bad-cb-amount`. - Unit suites `bls*` pass. On a revision that also carried #8095 and #8101, `coinjoin*`, `net_tests`, `miner_tests` and `validation*` passed as well. - The full functional suite (`test_runner.py -j16`), on the revision that still carried #8129 and not yet #8097 and #8131: 307 passed, 17 skipped, 1 failed. The skipped ones are the usual environment-dependent tests (`*_bind*`, `interface_usdt_*`, `interface_zmq*`, previous-release compatibility and `feature_unsupported_utxo_db.py`). The run includes `feature_llmq_dkgerrors.py` with #8104's new scenario and `interface_rest.py` with #8099's. - The failure was `feature_llmq_signing.py`, in its first `mine_quorum`, waiting for every member to have received all contributions. The node logs show one masternode relaying its contribution about 0.2 s before its connections to the other members had been authenticated, so no peer received it. That happens before anything #8135 or #8130 changes, and it is load-dependent: running four copies of the test in both variants at `-j8`, three times, 2 of 24 runs fail on that revision and 2 of 24 fail on the same branch without the two DKG commits, with the same assertion. - A full functional run with the #8097 and #8131 changes on top of an older revision of this branch: 310 passed, 17 skipped, none failed. #8131 describes its scenarios and how each was shown to fail without the change. The GUI picks (#8052, #8080) were not built or run locally; CI covers them. The first push of this branch did not compile (`m_special_tx`, see Adaptations); review caught it and it was fixed before any of the above was run. ## Breaking Changes None. Behaviour change for masternodes (#8131): a NO vote on a non-matching trigger may be cast later than before, when the masternode's own candidate has no trigger yet, and at the latest 3 blocks before the superblock. ## Checklist: - [x] I have performed a self-review of my own code - [ ] I have made corresponding changes to the documentation - [x] I have assigned this pull request to a milestone _(if you have permission to set one)_ 🤖 Generated with [Claude Code](https://claude.com/claude-code) Top commit has no ACKs. Tree-SHA512: 8ce635d02dd2b9b285aa6a1630d329ffeb9dcb99947a7a758c80ed17a2dbeb7496162a7f3052e872f46d481c6e6391ce866fd296d3ef1fa8edb1dc2d2c5f275e

knst added 2 commits November 12, 2026 15:11

bench: measure the DKG commit step at llmq_400_85 size

49d9ed5

Builds and verifies a premature commitment for 400 members and threshold 340. The cold variant first decodes every verification vector from its database encoding, as GetVerifiedContributions does on a cache miss.

knst requested review from PastaPastaPasta and UdjinM6 as code owners November 12, 2026 08:13

knst changed the title ~~perf(llmq): keep verified DKG contributions cached through the commit phase~~ perf: keep verified DKG contributions cached through the commit phase Nov 12, 2026

knst requested a review from thepastaclaw November 12, 2026 08:15

knst mentioned this pull request Nov 12, 2026

fix: process a node's own DKG messages ahead of queued peer messages #8130

Merged

2 of 3 tasks

knst added the backport-candidate-24.0.x label Nov 12, 2026

PastaPastaPasta mentioned this pull request Nov 12, 2026

chore: prepare v24.0.1 release #8133

Open

3 tasks done

knst modified the milestones: 24.1, 24 Nov 12, 2026

This was referenced Nov 12, 2026

backport: v24.0.x backports for v24.0.1 #8128

Merged

backport: wait for a matching superblock trigger before voting NO on the others (#8131) for v24.0.x #8132

Closed

thepastaclaw suggested changes Nov 12, 2026

View reviewed changes

thepastaclaw added the pastaclaw:changes-requested thepastaclaw's latest review requested changes label Nov 12, 2026

PastaPastaPasta mentioned this pull request Nov 12, 2026

fix: re-sign a pending EHF signal on every block so a failed first attempt can recover #8129

Closed

2 of 3 tasks

PastaPastaPasta reviewed Nov 12, 2026

View reviewed changes

PastaPastaPasta approved these changes Nov 12, 2026

View reviewed changes

PastaPastaPasta merged commit 1abaa0d into dashpay:develop Nov 12, 2026
55 of 65 checks passed

thepastaclaw removed the pastaclaw:changes-requested thepastaclaw's latest review requested changes label Nov 12, 2026

Navigation Menu

Repository navigation

perf: keep verified DKG contributions cached through the commit phase - #8135

perf: keep verified DKG contributions cached through the commit phase#8135
PastaPastaPasta merged 2 commits into
dashpay:developfrom
knst:perf-dkg-cache-verified-vvecs

knst commented Nov 12, 2026 •

edited

Loading

Uh oh!

thepastaclaw commented Nov 12, 2026 •

edited

Loading

Uh oh!

coderabbitai Bot commented Nov 12, 2026 •

edited

Loading

Uh oh!

PastaPastaPasta commented Nov 12, 2026

Uh oh!

thepastaclaw left a comment

Uh oh!

thepastaclaw Nov 12, 2026

Uh oh!

PastaPastaPasta Nov 12, 2026

Uh oh!

thepastaclaw Nov 12, 2026

Uh oh!

PastaPastaPasta Nov 12, 2026

Uh oh!

thepastaclaw Nov 12, 2026

Uh oh!

thepastaclaw Nov 12, 2026

Uh oh!

PastaPastaPasta left a comment

Uh oh!

PastaPastaPasta left a comment

Uh oh!

Uh oh!

Reviewers

Uh oh!

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

Conversation

knst commented Nov 12, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Issue being fixed or feature implemented

What was done?

Compared to #8130

How Has This Been Tested?

Measurements

Breaking Changes

Checklist:

Uh oh!

thepastaclaw commented Nov 12, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

coderabbitai Bot commented Nov 12, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Walkthrough

Uh oh!

PastaPastaPasta commented Nov 12, 2026

Uh oh!

thepastaclaw left a comment

Choose a reason for hiding this comment

Final validation — Phase 1 + Phase 2

Uh oh!

thepastaclaw Nov 12, 2026

Choose a reason for hiding this comment

Uh oh!

PastaPastaPasta Nov 12, 2026

Choose a reason for hiding this comment

Uh oh!

thepastaclaw Nov 12, 2026

Choose a reason for hiding this comment

Uh oh!

PastaPastaPasta Nov 12, 2026

Choose a reason for hiding this comment

Uh oh!

thepastaclaw Nov 12, 2026

Choose a reason for hiding this comment

Uh oh!

thepastaclaw Nov 12, 2026

Choose a reason for hiding this comment

Uh oh!

PastaPastaPasta left a comment

Choose a reason for hiding this comment

Uh oh!

PastaPastaPasta left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Uh oh!

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

knst commented Nov 12, 2026 •

edited

Loading

thepastaclaw commented Nov 12, 2026 •

edited

Loading

coderabbitai Bot commented Nov 12, 2026 •

edited

Loading